Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add datetime.date support to JSONEncoder #1326

Merged
merged 1 commit into from
Jan 23, 2015
Merged

Add datetime.date support to JSONEncoder #1326

merged 1 commit into from
Jan 23, 2015

Conversation

RealSalmon
Copy link

Add support for the datetime.date type to jsonify, which currently only supports the datetime.datetime type.

Changelog
  1. datetime.date type support in flask.json.jsonify

@davidism
Copy link
Member

datetime is a subclass of date, there's no need to check both. Simply isinstance(o, date) will suffice.

@RealSalmon
Copy link
Author

@davidism - thanks! I've reduced it to a single check.

@davidism
Copy link
Member

You should add a test for this as well. In addition, the commits can be squashed into one.

@untitaker
Copy link
Contributor

And please add a changelog too :)

You can squash commits with:

git rebase -i master

But it doesn't really matter if you don't.

@davidism Thanks for reviewing this!

@RealSalmon
Copy link
Author

Will do, thanks!

@RealSalmon
Copy link
Author

Added tests for both datetime.datetime and datetime.date types.

@untitaker
Copy link
Contributor

Could you add a changelog item for 1.0 too?

@RonnyPfannschmidt
Copy link
Contributor

why does the "unit-test" involve a flask app and a test client, its just jsonify this time
(i realize the surrounding tests are acceptance tests of request handling)

@RealSalmon
Copy link
Author

@untitaker - changelog entry added. sorry - I misunderstood what you were asking for before.

@RonnyPfannschmidt - mainly it's that way because that's how all of other tests are to be set up, but also because jsonify doesn't seem to work outside of an application context. I supposed because it returns an actual response in addition to the serialized data.

@untitaker untitaker merged commit 61263e0 into pallets:master Jan 23, 2015
untitaker added a commit that referenced this pull request Jan 23, 2015
@untitaker
Copy link
Contributor

Woo, thanks!

Your explanation for using a temporary app for testing is sound, although I haven't explored this myself.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants